Youtube search filters#124
Youtube search filters#124Bluesir9 wants to merge 21 commits intoTeamNewPipe:devfrom Bluesir9:youtube_search_filters
Conversation
- Added new logic to implement sort and search filters for YouTube search filters in YoutubeSearchQueryHandlerFactory. - Corrected some typos in SearchQueryHandlerFactory. - Changed test cases affected by changes made to YoutubeSearchQueryHandlerFactory. - Added todo items for the 2 primary test cases in YoutubeSearchQHTest.
- Added ability to handle more filters. - Added test cases to appropriately test the new filters implemented in combination with the available sorters.
|
You can find a description on how to test the changes on the frontend here: https://teamnewpipe.github.io/documentation/04_Run_changes_in_App/ |
…dded Base64 class in YoutubeSearchQueryHandlerFactory
|
@theScrabi Have tested the changes on the app and made necessary changes. |
|
All right thx :) |
TobiGr
left a comment
There was a problem hiding this comment.
@Bluesir9 I found some time to take a look at your changes. As said before, I am not that familiar with the extractor, but I'll try my best to give you detailed feedback. In case it sounds like nonsense, please tell me. There are so many largre PRs to review atm, so I might have missed something important.
I didn't test the changes yet, but the code looks quite mature.
I have one question about the added filters. You added following filters:
Time: last hour, today, this week, this month, this year
Type/Content: video, channel, playlist, movie, show,
Duration: short, long
Sorting: relevance, rating, upload date, view count
Do you intend to implement the features filter with
live, 4K, HD, Subtitles/CC, Creative Commons, 360°, VR180, 3D, HDR, location?
- Fixed indendation issue in YoutubeSearchQueryHandlerFactory and YoutubeSearchQHTest. - Added Features filter type and filter.
- Fixed handling for features content filters. - Added test cases to check for whether feature content filters are working fine.
|
@TobiGr For whatever reason "Feature" filters were not working with the changes I had done last time around, so in the interest of time I removed them last minute. I have implemented them now in the last couple of commits and its working fine. Have also added tests for the same. |
- Added license header to Base64 class. - Added Apache license for AOSP project in a different directory.
|
Not sure if you consider this good style, but let me assure you, it's not, it's the worst I've seen in quite a while, actually. The first line of your commit is what is visible from e.g., Please see https://chris.beams.io/posts/git-commit/ or https://dev.to/pavlosisaris/git-commits-an-effective-style-guide-2kkn how to write better commit messages. Also, please try to commit one change at a time, not multiple. Every time you need a bullet point list to describe your changes, you're probably trying to commit too much at once. This principle is also referred to as "atomic commits". See e.g., https://medium.com/@fagnerbrack/one-commit-one-change-3d10b10cebbf, for more information. |
|
@TheAssassin Point(s) taken. |
theScrabi
left a comment
There was a problem hiding this comment.
Please also swtich to dev branch.
| Movie(FilterType.Content,(byte)0x04), | ||
| Show(FilterType.Content,(byte)0x05), | ||
|
|
||
| Hour(FilterType.Time,(byte)0x01), |
There was a problem hiding this comment.
NewPipe can not handle key value pair filter yet :/
There was a problem hiding this comment.
I am not sure what you mean but this works out if the box. Albeit it looks kinda bad:
There was a problem hiding this comment.
Yes this is the issue i ment, it is not possible to add or remove filters like key values. This messes up the UI as you see. The UI should be made ready to handle these filters.
There was a problem hiding this comment.
Maybe we could implement the filters at 2 levels? We would initially ask the user to select a FilterType and after they have chosen that, we can ask them the exact filter they wish to apply falling under that FilterType?
So, clicking on the menu icon the first time, will load a list containing Content, Time, Duration, Feature. If the user chooses Duration, then we will show a second list containing Short, Long. And when the user chooses one of those options, then the search is triggered. Does that make sense?
There was a problem hiding this comment.
Let's finish this PR. 🎉 Thanks for being patient!
I am working on a proper design for the filters. This might take until next Sunday, because the new semesters is about to start and thus there are a bunch of other things to do. But I would not recommend to have endless lists. They come with bad UI/UX, because it is hard to get all the (important) information.
FYI:
There was a problem hiding this comment.
@TobiGr Of course, take your time. I was just surprised and happy that it worked functionally out of the box.
There was a problem hiding this comment.
I put my UI ideas into this issue in the app repo. I guess a UI discussion fits better into the other repo.
…ff Apache Commons Codec library
|
While testing I found that the app is throwing an error in case of some filters(4K only for now). I will look into this and potentially push fixes for the same by tomorrow. |
…se correct titles instead of name when matching elements.
|
Closing as it's based on YouTube's old site, introduces a new dependency which isn't neccessary, and there being no proper UI for it in NewPipe. Feel free to open a new PR in the future. |
Uh oh!
There was an error while loading. Please reload this page.